Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove ES3-specific transforms #12716

Merged
merged 1 commit into from
Apr 30, 2018
Merged

Remove ES3-specific transforms #12716

merged 1 commit into from
Apr 30, 2018

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Apr 28, 2018 •
edited
Loading

We stopped supporting IE8 a long time ago.
As far as I can tell all other major browsers support reserved property names.

By dropping these transforms we ensure the bundles contain fiber.return rather than fiber["return"] which I'm worried might deopt.

I've seen ["return"] being "hot" in some line-by-line Chrome traces before.

@klamping klamping mentioned this pull request Apr 28, 2018
Open
@pull-bot
Copy link

ReactDOM: size: -0.4%, gzip: -0.2%

Details of bundled changes.

Comparing: 045d4f1...52209cc

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js -0.1% -0.1% 611.9 KB 611.47 KB 141.37 KB 141.28 KB UMD_DEV
react-dom.production.min.js -0.4% -0.2% 100.42 KB 100.05 KB 31.89 KB 31.83 KB UMD_PROD
react-dom.development.js -0.1% -0.1% 596.28 KB 595.84 KB 137.23 KB 137.11 KB NODE_DEV
react-dom.production.min.js -0.4% -0.1% 98.86 KB 98.49 KB 31.09 KB 31.06 KB NODE_PROD
react-dom-test-utils.development.js -0.1% -0.1% 40.75 KB 40.71 KB 11.69 KB 11.68 KB UMD_DEV
react-dom-test-utils.production.min.js -0.4% -0.4% 10.64 KB 10.6 KB 3.96 KB 3.94 KB UMD_PROD
react-dom-test-utils.development.js -0.1% -0.1% 35.61 KB 35.57 KB 10.27 KB 10.26 KB NODE_DEV
react-dom-test-utils.production.min.js -0.4% -0.1% 9.91 KB 9.87 KB 3.73 KB 3.73 KB NODE_PROD
react-dom-unstable-native-dependencies.development.js -0.0% -0.0% 61.18 KB 61.18 KB 16.02 KB 16.02 KB UMD_DEV
react-dom-unstable-native-dependencies.production.min.js -0.0% -0.1% 11.32 KB 11.32 KB 3.92 KB 3.91 KB UMD_PROD
react-dom-unstable-native-dependencies.development.js -0.0% -0.0% 56.85 KB 56.84 KB 14.77 KB 14.77 KB NODE_DEV
react-dom-unstable-native-dependencies.production.min.js -0.0% -0.1% 10.66 KB 10.66 KB 3.68 KB 3.67 KB NODE_PROD
react-dom-server.browser.development.js -0.0% -0.1% 101.33 KB 101.29 KB 26.47 KB 26.45 KB UMD_DEV
react-dom-server.browser.production.min.js -0.2% -0.1% 15.2 KB 15.16 KB 5.81 KB 5.8 KB UMD_PROD
react-dom-server.browser.development.js -0.0% -0.1% 90.63 KB 90.59 KB 24.21 KB 24.18 KB NODE_DEV
react-dom-server.browser.production.min.js -0.2% -0.1% 14.56 KB 14.52 KB 5.55 KB 5.54 KB NODE_PROD
react-dom-server.node.development.js -0.0% -0.1% 92.55 KB 92.51 KB 24.75 KB 24.73 KB NODE_DEV
react-dom-server.node.production.min.js -0.2% -0.1% 15.36 KB 15.33 KB 5.84 KB 5.84 KB NODE_PROD
ReactDOM-dev.js -0.1% -0.0% 620.71 KB 620.28 KB 139.93 KB 139.87 KB FB_WWW_DEV
ReactDOM-prod.js -0.1% -0.1% 284.47 KB 284.15 KB 51.97 KB 51.93 KB FB_WWW_PROD
ReactTestUtils-dev.js -0.1% -0.1% 36.89 KB 36.85 KB 10.46 KB 10.45 KB FB_WWW_DEV
ReactDOMUnstableNativeDependencies-dev.js -0.0% -0.0% 57.09 KB 57.09 KB 14.56 KB 14.56 KB FB_WWW_DEV
ReactDOMUnstableNativeDependencies-prod.js -0.0% -0.1% 26.34 KB 26.34 KB 5.38 KB 5.37 KB FB_WWW_PROD
ReactDOMServer-dev.js -0.0% -0.0% 94.1 KB 94.06 KB 24.02 KB 24.02 KB FB_WWW_DEV
ReactDOMServer-prod.js -0.2% -0.2% 31.62 KB 31.57 KB 7.78 KB 7.76 KB FB_WWW_PROD

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-art.development.js -0.1% -0.0% 414.82 KB 414.43 KB 90.24 KB 90.2 KB UMD_DEV
react-art.production.min.js -0.4% -0.2% 90.74 KB 90.38 KB 27.55 KB 27.49 KB UMD_PROD
react-art.development.js -0.1% -0.1% 340.66 KB 340.27 KB 71.6 KB 71.52 KB NODE_DEV
react-art.production.min.js -0.6% -0.1% 55.26 KB 54.91 KB 16.78 KB 16.76 KB NODE_PROD
ReactART-dev.js -0.1% -0.1% 348.92 KB 348.52 KB 71.13 KB 71.1 KB FB_WWW_DEV
ReactART-prod.js -0.2% -0.1% 167.21 KB 166.93 KB 27.45 KB 27.43 KB FB_WWW_PROD

react-test-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-test-renderer.development.js -0.1% -0.1% 348.75 KB 348.33 KB 73.3 KB 73.25 KB UMD_DEV
react-test-renderer.production.min.js -0.7% -0.2% 55.28 KB 54.89 KB 16.75 KB 16.72 KB UMD_PROD
react-test-renderer.development.js -0.1% -0.1% 339.58 KB 339.16 KB 70.62 KB 70.54 KB NODE_DEV
react-test-renderer.production.min.js -0.7% -0.2% 54.51 KB 54.13 KB 16.32 KB 16.28 KB NODE_PROD
react-test-renderer-shallow.development.js -0.2% -0.1% 23.95 KB 23.91 KB 6.39 KB 6.39 KB UMD_DEV
react-test-renderer-shallow.production.min.js -0.5% -0.1% 7.17 KB 7.13 KB 2.34 KB 2.34 KB UMD_PROD
react-test-renderer-shallow.development.js -0.2% -0.2% 13.71 KB 13.69 KB 3.43 KB 3.43 KB NODE_DEV
react-test-renderer-shallow.production.min.js -0.3% -0.3% 6.92 KB 6.9 KB 2.26 KB 2.25 KB NODE_PROD
ReactTestRenderer-dev.js -0.1% -0.1% 348.11 KB 347.68 KB 70.23 KB 70.18 KB FB_WWW_DEV
ReactShallowRenderer-dev.js -0.2% -0.2% 14.41 KB 14.39 KB 3.55 KB 3.54 KB FB_WWW_DEV

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js -0.1% -0.1% 319.91 KB 319.52 KB 66.07 KB 65.99 KB NODE_DEV
react-reconciler.production.min.js -0.8% -0.3% 47.36 KB 47 KB 14.25 KB 14.22 KB NODE_PROD
react-reconciler-persistent.development.js -0.1% -0.1% 319.25 KB 318.86 KB 65.83 KB 65.76 KB NODE_DEV
react-reconciler-persistent.production.min.js -0.7% -0.3% 46.28 KB 45.97 KB 14.14 KB 14.09 KB NODE_PROD
react-reconciler-reflection.development.js -0.7% -0.3% 11.06 KB 10.99 KB 3.42 KB 3.41 KB NODE_DEV
react-reconciler-reflection.production.min.js -2.4% -0.6% 2.32 KB 2.27 KB 1.02 KB 1.01 KB NODE_PROD

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js -0.1% -0.1% 459.73 KB 459.32 KB 98.28 KB 98.22 KB RN_FB_DEV
ReactNativeRenderer-prod.js -0.1% -0.1% 218.18 KB 217.88 KB 36.43 KB 36.39 KB RN_FB_PROD
ReactNativeRenderer-dev.js -0.1% -0.1% 459.48 KB 459.07 KB 98.21 KB 98.16 KB RN_OSS_DEV
ReactNativeRenderer-prod.js -0.1% -0.1% 217.4 KB 217.1 KB 36.3 KB 36.26 KB RN_OSS_PROD
ReactFabric-dev.js -0.1% -0.1% 442.14 KB 441.73 KB 93.88 KB 93.82 KB RN_FB_DEV
ReactFabric-prod.js -0.1% -0.1% 202.99 KB 202.77 KB 33.69 KB 33.67 KB RN_FB_PROD
ReactFabric-dev.js -0.1% -0.1% 442.18 KB 441.76 KB 93.89 KB 93.83 KB RN_OSS_DEV
ReactFabric-prod.js -0.1% -0.1% 203.03 KB 202.81 KB 33.71 KB 33.69 KB RN_OSS_PROD

Generated by 🚫 dangerJS

trueadm
trueadm approved these changes Apr 30, 2018
View reviewed changes
Copy link
Contributor

@trueadm trueadm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, if our concern is the name return being used as Fiber property – we should probably just use a different name internally. That's a bigger change though, but if there's a significant perf loss from this, then we should look more into it.

@gaearon
Copy link
Collaborator Author

gaearon commented Apr 30, 2018

I don't think it's the name specifically that's the issue—just the fact that it turns dot-access into quotes because of legacy browsers we don't care about anymore.

@gaearon
Copy link
Collaborator Author

gaearon commented Apr 30, 2018

The goal of this change isn't to fix that specifically, it's just that these transforms aren't useful now and can be removed.

@gaearon gaearon merged commit 9a9f547 into facebook:master Apr 30, 2018
@gaearon gaearon deleted the no-es3 branch April 30, 2018 13:30
@Simek
Copy link
Contributor

Simek commented May 10, 2018 •
edited
Loading

Should package.json be also cleaned up?

/~https://github.com/facebook/react/blob/master/package.json#L34

@gaearon
Copy link
Collaborator Author

gaearon commented May 10, 2018

Oh yeah, send a PR please?

Simek added a commit to Simek/react that referenced this pull request May 14, 2018
@Simek
remove unused ES3-specific packages - refs facebook#12716
9a74a99
Simek added a commit to Simek/react that referenced this pull request May 14, 2018
@Simek
remove unused ES3-specific packages - refs facebook#12716
9bb4454
@Simek Simek mentioned this pull request May 14, 2018
Merged
gaearon pushed a commit that referenced this pull request May 14, 2018
@Simek @gaearon
remove unused ES3-specific packages - refs #12716 (#12797)
8506062
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trueadm trueadm trueadm approved these changes

@sophiebits sophiebits Awaiting requested review from sophiebits

@sebmarkbage sebmarkbage Awaiting requested review from sebmarkbage

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gaearon @pull-bot @Simek @trueadm @facebook-github-bot